Repository navigation
[core] Keep the winning cell by reference in pick-value aggregators - #1338
Open
JeonDaehong wants to merge 1 commit into
Open
JeonDaehong wants to merge 1 commit into
JeonDaehong wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: part of #1336
last_value,first_value,last_non_null_value,first_non_null_valueandprimary-keyhold the current winner as a one-row slice and take a new slice for every input row. Columns without an aggregate function default tolast_non_null_value, so almost every aggregation table pays this per row.Brief change log
FieldAggregator::agg_array(&ArrayRef, row)defaults toagg. The pick aggregators override it to keep the input array and row index, andresult()slices once.merge_orderedand the partial-update aggregator path pass the column as anArrayRef. Columns without an aggregator keep the existing path.PK table, 2M keys written in 3 overlapping rounds,
sumon one column, full read (release, in-memory FileIO): 2,105–2,135 → 1,827–1,856 ms, measured in one binary with the old and new call paths switched by an env var, 3 alternating rounds. Allocations per merged key go from 10.2 to 8.2. Deduplicate and partial-update are unchanged.Tests
agg_array_matches_agg_for_every_policy: each pick policy gives the same result throughagg_arrayandagg, with NULLs among the inputs.agg_array_winner_feeds_reversed_and_delete_paths:agg_reversedandreplace_with_deleteafteragg_array.Ran the ci.yml steps locally (fmt, clippy, msrv, unit, rest-server, integration, datafusion, python, go).
API and Format
None.
agg_arrayis crate-internal.Documentation
None.